Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI icons re-design #1361

Merged
merged 93 commits into from
Oct 16, 2023
Merged

UI icons re-design #1361

merged 93 commits into from
Oct 16, 2023

Conversation

Fatih20
Copy link
Contributor

@Fatih20 Fatih20 commented May 26, 2020

All of the icons have been redesigned (needed to be refined and still prone to changes).
All of the icons name have been standardized. x-y instead of xY or x_y
All of the icons have been compartmentalized into folders.

closes #1356

@Fatih20
Copy link
Contributor Author

Fatih20 commented May 26, 2020

You can download the branch yourself (git clone -b Action-Icons Redesign https://github.com/Fatih20/pencil.git) and compile it to try the new icons firsthand and provide feedbacks.

@chchwy chchwy changed the title Action icons redesign Action icons redesign [WIP] May 27, 2020
@J5lx J5lx added the UI Related to the visual appearance of the program label Jun 2, 2020
@Jose-Moreno Jose-Moreno mentioned this pull request Sep 16, 2020
@J5lx J5lx marked this pull request as draft December 18, 2020 16:18
- We now have classic, legacy and playful
- Playful icons has been optimized and tweaked.
- Move icons into theme folders

I should have done the move separately but well... I forgot! and now it's done P:
@J5lx J5lx linked an issue Aug 9, 2023 that may be closed by this pull request
6 tasks
@J5lx J5lx added this to the v0.6.7 milestone Aug 9, 2023
@MrStevns
Copy link
Member

MrStevns commented Oct 1, 2023

At long last, this PR is ready to be reviewed!

What are your thoughts on keeping the old icons?
For the sake of potentially being able to include the old ones again as a theme pack, the icons have been separated into theme folders:

  • Legacy (old pencil)
  • Classic (our current)
  • Playful (the new design)

although we're currently using a combination of Legacy and Classic icons, I still thought it made sense to separate them. Should we keep or remove them?


Thanks to @Fatih20 for starting this re-design, hopefully my involvement and the changes I've made during the lifetime of this PR, hasn't made you less proud of what you started and achieved.

@J5lx J5lx self-assigned this Oct 1, 2023
Copy link
Member

@J5lx J5lx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn’t come as too much of a surprise after the discussion in the linked issue, but I’m already very satisfied with the state of this PR. Just a handful of small remarks at this point. Thanks for all the hard work on this!

What are your thoughts on keeping the old icons?

I think we should delete them. Should we ever have need of them again, we can simply recover them from git history.

app/src/mainwindow2.cpp Outdated Show resolved Hide resolved
app/ui/mainwindow2.ui Show resolved Hide resolved
app/ui/mainwindow2.ui Show resolved Hide resolved
app/data/icons/themes/playful/menubar/zoom-select.svg Outdated Show resolved Hide resolved
@Jose-Moreno
Copy link
Member

Jose-Moreno commented Oct 1, 2023

At long last, this PR is ready to be reviewed!

What are your thoughts on keeping the old icons? For the sake of potentially being able to include the old ones again as a theme pack, the icons have been separated into theme folders:

  • Legacy (old pencil)
  • Classic (our current)
  • Playful (the new design)

although we're currently using a combination of Legacy and Classic icons, I still thought it made sense to separate them. Should we keep or remove them?

Thanks to @Fatih20 for starting this re-design, hopefully my involvement and the changes I've made during the lifetime of this PR, hasn't made you less proud of what you started and achieved.

Bundling or Download?

I like the idea of modular theme packs. Are we hosting them separately in the github repo tho? or are we bundling them with Pencil2D.

We could either bundle classic & "playful", leaving legacy as an optional download (Github). However I recall back then, when the original icons were changed some people did lament the icon changes. It might happen again so it could be simpler to bundle them 🤔

One minor issue

There's only one think I didn't know If I should say, but It still bothers me, and it's the hand & finger icons in the playful version. I simply never liked it from the start.

If anyone reads the conversation, I tried to nudge the OP into changing the initial version, which he agreed to, but it was never possible to come to final solution after the project was halted.

The reasons are many but mostly I want to change the shape, proportion, anatomy and visual clarity by simplifying the details both have. In that sense the Classic ones are more readable.

How to contribute to the icon packs?

Right now I don't have the time to change it, but I'll ask for reference: If I did change the SVGs, would that be easily replaced in the icon bundle via PR to contribute a potential change?

Additionally how would themes / packs work? I think with this we should consider at least some minor form of documentation so we can provide that to our users 🤔

Thank you

Other than that everything looks very vibrant and focused, so I think you did a great job taking over the overall project. Thank you 🙇‍♂️

@J5lx
Copy link
Member

J5lx commented Oct 1, 2023

I like the idea of modular theme packs.

Just to be clear, there are no theme packs in this PR. Oliver only organised the icons into folders within the repository so that we could potentially add theme packs in the future, but right now the icon paths used by the program are still hard-coded. Theme packs are a distinct feature which is out-of-scope for this PR.

@MrStevns
Copy link
Member

MrStevns commented Oct 2, 2023

There's only one think I didn't know If I should say, but It still bothers me, and it's the hand & finger icons in the playful version. I simply never liked it from the start.
If anyone reads the conversation, I tried to nudge the OP into changing the initial version, which he agreed to, but it was never possible to come to final solution after the project was halted.

I remember that conversation but to be fair, both the smudge and the hand icon have been redesigned several times since then though, so just to be clear, we're talking about the latest iteration, right?

image

In that case let's go back to the drawing board with those and improve upon them until we're satisfied.

@Jose-Moreno
Copy link
Member

image

image

There's only one think I didn't know If I should say, but It still bothers me, and it's the hand & finger icons in the playful version. I simply never liked it from the start.
If anyone reads the conversation, I tried to nudge the OP into changing the initial version, which he agreed to, but it was never possible to come to final solution after the project was halted.

I remember that conversation but to be fair, both the smudge and the hand icon have been redesigned several times since then though, so just to be clear, we're talking about the latest iteration, right?

image In that case let's go back to the drawing board with those and improve upon them until we're satisfied.

Sorry, I forced myself to make some time to at least help. I don't want this project to be stopped bc of my comment either.

Here are some roughs for the design:
🔴 Red: Construction lines
⚫ Black: Intended outline
🔵 Blue: Optional details

Note: Please asume the construction lines to be perfectly straight lines & the circle modules are 1:1 in proportion. Finger caps are based off circles and fingers all have the same width.

I just think the base should be more modular, and then we can adapt that into the "playful" style with rounded corners. The axial guidelines for the fingers and the overlap between them in both should provide sufficient indication of perspective and the design of the "smugdes, should also be based in circular or ellipsoidal negative space (as if you cut the shapes with a boolean vector circle)

Additionally the glove details saturate the icon. If they were to e kept they could be a lighter color or we could change the pattern (horizontal instead of vertical, so it does not create a moire effect at low resolution, but ultimately if it doesn't work, I would propose to remove those lines on the back of the hands.

Thank you for taking the time to review this, and apologies I didn't mention it sooner.

@MrStevns
Copy link
Member

MrStevns commented Oct 3, 2023

Thanks for the help, I appreciate it 🙇

Meanwhile in order to not stall the PR further as you say, we'll let Jakob continue his review and merge the PR when it's done.

I'll then create a second PR later to continue the work on hand and smudge icon.

Copy link
Member

@J5lx J5lx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the remarks from my previous review all addressed, I don’t think I have much else to add. If anything, perhaps it would make sense to flip the zoom icons so the little overlay +/- is in the same corner as the layer/frame management icons. But as you say, maybe it is time to just merge this and make separate PRs for further improvements. So if no one else has anything to add, I think it’s fine to merge this.

What about the removal of the old icons, though? As I said before, I don’t think it’s worth keeping them around if we’re not going to use them anymore. If we do need them again, we can retrieve them from git history.

@MrStevns
Copy link
Member

perhaps it would make sense to flip the zoom icons so the little overlay +/- is in the same corner as the layer/frame management icons

The placement was deliberate as the overlays would obscure almost the entire handle, which is imo. important to decipher the icons properly. I did end up making the overlays slightly bigger though.

What about the removal of the old icons, though? As I said before, I don’t think it’s worth keeping them around if we’re not going to use them anymore. If we do need them again, we can retrieve them from git history.

Good point. I've removed them now.

@J5lx
Copy link
Member

J5lx commented Oct 15, 2023

the overlays would obscure almost the entire handle

My original thought was to flip the icons… but on second thought, maybe that would be a bit unusual because it would suggest left-handed usage…

@MrStevns
Copy link
Member

the overlays would obscure almost the entire handle

My original thought was to flip the icons… but on second thought, maybe that would be a bit unusual because it would suggest left-handed usage…

I considered it but decided against it for the reason that the majority of magnifier icons I could find were turned to the left rather than right.

@MrStevns
Copy link
Member

With Jose being onboard with the redesigned hand and smudge icons, and Jacob having reviewed the PR, I will merge the PR 🎉

@MrStevns MrStevns merged commit 6a4fe12 into pencil2d:master Oct 16, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement UI Related to the visual appearance of the program
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

I'm interested in redesigning all icons in the software Improve support for dark themes
4 participants